Skip to content

Improve and fix mpsc documentation #37941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 13, 2016
Merged

Improve and fix mpsc documentation #37941

merged 1 commit into from
Dec 13, 2016

Conversation

Cobrand
Copy link
Contributor

@Cobrand Cobrand commented Nov 22, 2016

Closes #37915

This commit enhances documentation with several links and
fixes an error in the sync_channel documentation as well:
send doesn't panic when the senders are all disconnected

r? @steveklabnik

@mfarrugi
Copy link

Are the references to 'runtime' also outdated? eg. L235

@Cobrand
Copy link
Contributor Author

Cobrand commented Nov 25, 2016

@mfarrugi I don't really get what you mean ... could you explain more in detail ?

@mfarrugi
Copy link

I'm not clear on what runtime refers to in
// A major goal of these channels is to work seamlessly on and off the runtime.
But I think it refers to the old rust runtime

@Cobrand
Copy link
Contributor Author

Cobrand commented Nov 25, 2016

Indeed it looks like the mention of the runtime is part of the old documentation that was kept here.

A quick git blame told me that most the the comments in this file were from 2013 or 2014, so definitely way before the first release of Rust.

That might need another PR though ?

@bors
Copy link
Collaborator

bors commented Nov 26, 2016

☔ The latest upstream changes (presumably #38015) made this pull request unmergeable. Please resolve the merge conflicts.

@Cobrand
Copy link
Contributor Author

Cobrand commented Dec 6, 2016

This has been opened for several days but I don't see any issue, I guess I'll try my luck with someone else ?
r? @GuillaumeGomez

GuillaumeGomez
GuillaumeGomez requested changes Dec 7, 2016
View reviewed changes
src/libstd/sync/mpsc/mod.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All functions/methods name should have () at the end.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

src/libstd/sync/mpsc/mod.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra whitespace before "rendezvous". By the way, shouldn't it be rendez-vous? French in english is so strange haha.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and same comment for send in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Being french myself I had my doubts about that but in English it's definitely rendezvous :

https://fr.wikipedia.org/wiki/Rendez-vous
https://en.wikipedia.org/wiki/Rendezvous

For some unknown and I guess historical reasons the translators dropped the dash

src/libstd/sync/mpsc/mod.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment for send.

@Cobrand
Improve and fix mpsc documentation
57f998a 
Closes rust-lang#37915

This commit enhances documentation with several links and
fixes an error in the `sync_channel` documentation as well:
`send` doesn't panic when the senders are all disconnected
@Cobrand Cobrand force-pushed the docfix-issue-37915 branch from b4db1a7 to 57f998a Compare December 7, 2016 17:57
@GuillaumeGomez
Copy link
Member

All good, thanks!

@bors: r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 7, 2016

📌 Commit 57f998a has been approved by GuillaumeGomez

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Dec 12, 2016
@frewsxcv
Rollup merge of rust-lang#37941 - Cobrand:docfix-issue-37915, r=Guill…
0b815c2 
…aumeGomez

Improve and fix mpsc documentation

Closes rust-lang#37915

This commit enhances documentation with several links and
fixes an error in the `sync_channel` documentation as well:
`send` doesn't panic when the senders are all disconnected

r? @steveklabnik
@frewsxcv frewsxcv mentioned this pull request Dec 12, 2016
Closed
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Dec 12, 2016
@frewsxcv
Rollup merge of rust-lang#37941 - Cobrand:docfix-issue-37915, r=Guill…
8c3c9c2 
…aumeGomez

Improve and fix mpsc documentation

Closes rust-lang#37915

This commit enhances documentation with several links and
fixes an error in the `sync_channel` documentation as well:
`send` doesn't panic when the senders are all disconnected

r? @steveklabnik
@frewsxcv frewsxcv mentioned this pull request Dec 12, 2016
Merged
bors added a commit that referenced this pull request Dec 13, 2016
@bors
Auto merge of #38325 - frewsxcv:rollup, r=frewsxcv
5a2b50b 
Rollup of 7 pull requests

- Successful merges: #37052, #37941, #38067, #38164, #38202, #38264, #38299
- Failed merges:
@bors bors merged commit 57f998a into rust-lang:master Dec 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuillaumeGomez GuillaumeGomez GuillaumeGomez approved these changes

Assignees

@GuillaumeGomez GuillaumeGomez

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Documentation phrase about panics in mpsc channel may be obsolete
5 participants
@Cobrand @mfarrugi @bors @GuillaumeGomez @steveklabnik